Skip to content

Quality: Stack overflow risk in bytesToBase64 due to large spread operator#520

Open
tomaioo wants to merge 1 commit into
codedogQBY:mainfrom
tomaioo:improve/quality/stack-overflow-risk-in-bytestobase64-due
Open

Quality: Stack overflow risk in bytesToBase64 due to large spread operator#520
tomaioo wants to merge 1 commit into
codedogQBY:mainfrom
tomaioo:improve/quality/stack-overflow-risk-in-bytestobase64-due

Conversation

@tomaioo

@tomaioo tomaioo commented Jul 3, 2026

Copy link
Copy Markdown

Summary

Quality: Stack overflow risk in bytesToBase64 due to large spread operator

Problem

Severity: High | File: packages/app-expo/src/lib/rag/auto-vectorize-book.ts:L33

The bytesToBase64 function spreads a Uint8Array chunk of size 0x8000 (32768) into String.fromCharCode(...). Most JavaScript engines have a maximum argument count limit (typically ~65536 but often lower in practice), and spreading such a large array will cause a 'Maximum call stack size exceeded' error for files larger than ~32KB.

Solution

Replace the spread operator with a loop or use a smaller chunk size (e.g., 0x1000 or 4096). Alternatively, use Array.from(chunk, b => String.fromCharCode(b)).join('') or a TextDecoder-based approach to avoid stack overflow.

Changes

  • packages/app-expo/src/lib/rag/auto-vectorize-book.ts (modified)

The `bytesToBase64` function spreads a `Uint8Array` chunk of size `0x8000` (32768) into `String.fromCharCode(...)`. Most JavaScript engines have a maximum argument count limit (typically ~65536 but often lower in practice), and spreading such a large array will cause a 'Maximum call stack size exceeded' error for files larger than ~32KB.

Signed-off-by: tomaioo <[email protected]>
@codedogQBY

Copy link
Copy Markdown
Owner

中文:这个修复方向是对的,但目前只改了 packages/app-expo/src/lib/rag/auto-vectorize-book.ts 里的 bytesToBase64。同样的 String.fromCharCode(...chunk) 还存在于 packages/app-expo/src/stores/library-store.tspackages/app-expo/src/screens/LibraryScreen.tsx,这些移动端导入/回退提取路径仍然可能在大文件或大字节数组上触发 Maximum call stack size exceeded。建议把 base64 转换抽成一个安全 helper,或者至少同步修掉所有 call sites 后再合并。

English: The fix is in the right direction, but it only updates bytesToBase64 in packages/app-expo/src/lib/rag/auto-vectorize-book.ts. The same String.fromCharCode(...chunk) pattern still exists in packages/app-expo/src/stores/library-store.ts and packages/app-expo/src/screens/LibraryScreen.tsx, so mobile import/fallback extraction paths may still hit Maximum call stack size exceeded for large byte arrays. Please centralize this into a safe base64 helper or update all call sites before merge.

@codedogQBY codedogQBY left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

中文:这个 PR 方向是对的,但现在还不能合。它只修了 ,同样的 仍然存在于 和 ,移动端导入/回退提取路径仍可能触发同类参数列表溢出。建议抽一个共享的 safe bytes/base64 helper,使用普通循环分块转换,统一替换这些 call sites;这样也能避免 每块额外创建大量中间字符串数组。

English: The direction is right, but this is not ready to merge yet. It only fixes ; the same pattern still exists in and , so mobile import/fallback extraction can still hit the same argument-list overflow. Please centralize this into a shared safe bytes/base64 helper using a normal loop over chunks, and replace all affected call sites. That would also avoid the extra intermediate string array created by .

@codedogQBY codedogQBY left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

中文:这个 PR 方向是对的,但现在还不能合。它只修了 packages/app-expo/src/lib/rag/auto-vectorize-book.ts,同样的 String.fromCharCode(...chunk) 仍然存在于 packages/app-expo/src/stores/library-store.tspackages/app-expo/src/screens/LibraryScreen.tsx,移动端导入/回退提取路径仍可能触发同类参数列表溢出。建议抽一个共享的 safe bytes/base64 helper,使用普通循环分块转换,统一替换这些 call sites;这样也能避免 Array.from(chunk, ...).join("") 每块额外创建大量中间字符串数组。

English: The direction is right, but this is not ready to merge yet. It only fixes packages/app-expo/src/lib/rag/auto-vectorize-book.ts; the same String.fromCharCode(...chunk) pattern still exists in packages/app-expo/src/stores/library-store.ts and packages/app-expo/src/screens/LibraryScreen.tsx, so mobile import/fallback extraction can still hit the same argument-list overflow. Please centralize this into a shared safe bytes/base64 helper using a normal loop over chunks, and replace all affected call sites. That would also avoid the extra intermediate string array created by Array.from(chunk, ...).join("").

@codedogQBY
codedogQBY dismissed their stale review July 5, 2026 14:43

Dismiss malformed duplicate review caused by shell quoting; keeping the complete follow-up review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants